Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): don't delete offline files from disk when trash empties #14777

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

etnoy
Copy link
Contributor

@etnoy etnoy commented Dec 18, 2024

Right now, immich deletes files after 30 days that are excluded or outside of import paths, including original files.

This is not intended, if a new exclusion pattern is added or an import path is changed and one or more assets are removed due to it, we should not be deleting original files.

This bug is in my opinion quite serious and could cause data loss.

I can't find the original bug report (github? reddit?)

Edit: This might be it: https://www.reddit.com/r/immich/comments/1gu61r3/immich_tried_to_remove_my_external_libraries_file/

Also adds some e2e tests related to offline files

@etnoy etnoy force-pushed the fix/empty-offline-files branch from 91b3b6a to 0cffb62 Compare December 18, 2024 21:38
@etnoy etnoy force-pushed the fix/empty-offline-files branch from 0cffb62 to 1a6e9bc Compare December 18, 2024 22:23
@etnoy etnoy force-pushed the fix/empty-offline-files branch from 1a6e9bc to e8c5fcf Compare December 18, 2024 22:30
Copy link
Contributor

@zackpollard zackpollard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good overall, I think just the point Jason raised, the asset service isn't where that logic should live

@etnoy
Copy link
Contributor Author

etnoy commented Dec 19, 2024

This looks good overall, I think just the point Jason raised, the asset service isn't where that logic should live

The logic still needs to be in the asset service since that's where the deletion is initiated. Ideally I feel like it could be in the trash service, but it isn't for the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants